Enum cycle in an undirected graph (and fix bug in yen_k_shortest_simple_path algorithm) by kappybar · Pull Request #40217 · sagemath/sage

kappybar · 2025-06-06T12:34:04Z

Add implementation of enumeration of cycles in an undirected graph.
Specifically, I make sage/graphs/cycle_enumeration.py and modify

~sage.graphs.cycle_enumeration._all_simple_cycles_iterator_edge
~sage.graphs.cycle_enumeration.all_cycles_iterator
~sage.graphs.cycle_enumeration.all_simple_cycles

In implementing these funcionts, I fix bug in ~sage.graphs.path_enumeration.yen_k_shortest_simple_paths.
This bug occurs when there is no path between source and target.

Also, I add ~sage.graphs.connectivity.biconnected_components_subgraphs.

📝 Checklist

The title is concise and informative.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation and checked the documentation preview.

⌛ Dependencies

#40248

…o this implementation)

github-actions · 2025-06-06T13:54:08Z

Documentation preview for this PR (built with commit 0418243; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

dcoudert · 2025-06-09T09:54:17Z

    return g

+def biconnected_components_subgraphs(G):
+    """


It's best to use r""" here.

dcoudert · 2025-06-09T10:09:04Z

+                raise ValueError("negative weight is not allowed")
+
+    if algorithm == 'A':
+        if not self.is_directed():


I think that checks on input parameters should be done before any call to copy or decomposition into biconnected components.

dcoudert · 2025-06-09T10:28:51Z

+    if starting_vertices is None:
+        starting_vertices = self
+
+    if self.is_directed():


I think we can avoid this part of the code

if self.is_directed(): # Since a cycle is always included in a given strongly connected # component, we may remove edges from the graph sccs = self.strongly_connected_components() d = {} for id, component in enumerate(sccs): for v in component: d[v] = id h = copy(self) h.delete_edges((u, v) for u, v in h.edge_iterator(labels=False) if d[u] != d[v]) else: h = copy(self)

Indeed, for algorithm 'B', the first step is to decompose the graph h into strongly connected / biconnected components. So there is no added value to make a copy of self and/or remove it some edges.

It might be different for algorithm 'A', but we must be careful. Notice that a cut vertex may belong to several strongly connected components, so we cannot decompose the graph into strongly connected components first. We can remove arcs between components. So may be this part could be moved into the block for algorithm 'A'.

dcoudert · 2025-06-09T10:30:16Z

+        h = None
+        for component in components:
+            if component.has_edge(edge):
+                h = component


add a break to exist the loop early. Actually, you can use (without h = None):

for component in components: if component.has_edge(edge): h = component break else: # edge connects two strongly connected components, so # no simple cycle starting with edge exists. return

dcoudert

otherwise, LGTM.

dcoudert · 2025-06-10T08:58:22Z

+            yield [vertex]
+    # First we remove vertices and edges that are not part of any cycle
+    if remove_acyclic_edges:
+        sccs = self.strongly_connected_components()


further minor improvement: if the graph is strongly connected (so len(sccs) == 1), we can do h = self.

dcoudert · 2025-06-10T09:01:28Z

+            # Since a cycle is always included in a given strongly connected
+            # component, we may remove edges from the graph
+            sccs = self.strongly_connected_components()
+            d = {}


here also, if the graph is strongly connected, there is no need for a copy.

dcoudert

I don't think it is necessary to expose methods _all_cycles_iterator_vertex and _all_simple_cycles_iterator_edge in DiGraph. These are internal method users can always import these methods if needed. Of course, if these methods are not exposed, you will have to import them in your methods and in relevant doctests.

dcoudert · 2025-06-15T08:33:42Z

in the description of this PR, you must add the dependency on #40248.

kappybar · 2025-06-15T11:05:56Z

If I stop exposing method _all_cycles_iterator_vertex or _all_simple_cycles_iterator_edge in Digraph,
I have to change

h._all_cycles_iterator_vertex(v,
                                                 starting_vertices=starting_vertices,
                                                 simple=simple,
                                                 rooted=rooted,
                                                 max_length=max_length,
                                                 trivial=trivial,
                                                 remove_acyclic_edges=False,
                                                 weight_function=weight_function,
                                                 by_weight=by_weight,
                                                 check_weight=check_weight,
                                                 report_weight=True)

to

_all_cycles_iterator_vertex(h, v,
                                                 starting_vertices=starting_vertices,
                                                 simple=simple,
                                                 rooted=rooted,
                                                 max_length=max_length,
                                                 trivial=trivial,
                                                 remove_acyclic_edges=False,
                                                 weight_function=weight_function,
                                                 by_weight=by_weight,
                                                 check_weight=check_weight,
                                                 report_weight=True)

for each function call.
Do you mean this change?

dcoudert · 2025-06-15T11:12:18Z

Yes, this is what I mean, but I don't know if it is a good idea or not... Well, let it as is. It's working.

kappybar · 2025-06-15T12:40:10Z

I cannot determine which is good neither ... It might be fine to leave the current implementation as it is.

dcoudert

ok, let's go for this version. Thanks.

sagemathgh-40217: Enum cycle in an undirected graph (and fix bug in yen_k_shortest_simple_path algorithm)       Add implementation of enumeration of cycles in an undirected graph. Specifically, I make ```sage/graphs/cycle_enumeration.py``` and modify + ```~sage.graphs.cycle_enumeration._all_simple_cycles_iterator_edge``` + ```~sage.graphs.cycle_enumeration.all_cycles_iterator``` + ```~sage.graphs.cycle_enumeration.all_simple_cycles``` In implementing these funcionts, I fix bug in ```~sage.graphs.path_enumeration.yen_k_shortest_simple_paths```. This bug occurs when there is no path between source and target. Also, I add ```~sage.graphs.connectivity.biconnected_components_subgraphs```. ### 📝 Checklist  - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies sagemath#40248    URL: sagemath#40217 Reported by: Yuta Inoue Reviewer(s): David Coudert

sagemathgh-40284: PNC k shortest simple path (for directed graphs)       Implement PNC k shortest simple path for directed graphs. I implemented for only directed graphs for simplicity first. ### 📝 Checklist  - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. - [x] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies    sagemath#40248 sagemath#40217 URL: sagemath#40284 Reported by: Yuta Inoue Reviewer(s): David Coudert

kappybar added 4 commits June 3, 2025 22:38

create a new file related to cycle enumeration

b051386

fix format

9ad0fb3

implment cycle enumeration in undirected graph (and fix bug related t…

573942d

…o this implementation)

add cycle_enumeration.pyx in meson.build

31ce4bc

add cycle_enumeration in index.rst in Documents

8f3188b

dcoudert added s: needs review c: graph theory labels Jun 9, 2025

dcoudert requested changes Jun 9, 2025

View reviewed changes

kappybar added 2 commits June 9, 2025 20:53

fix format and move code

f09c6e7

remove unnecessary comment

d45b52e

dcoudert reviewed Jun 10, 2025

View reviewed changes

skip copy if unnecessary

9057ffd

dcoudert added the gsoc: 2025 Tag for GSoC2025 issues/PRs label Jun 11, 2025

report weight if s=t in yen_k_shortest_simple_paths

e8362fd

dcoudert mentioned this pull request Jun 12, 2025

Fix reporting weight in yen_k_shortest_simple_paths #40248

Merged

5 tasks

change to shorter code

f2ef353

kappybar force-pushed the enum_cycle_undirected branch from 4b4a3e3 to 9057ffd Compare June 15, 2025 01:22

kappybar and others added 4 commits June 15, 2025 10:38

Merge branch 'develop' into enum_cycle_undirected

cde669f

Merge branch 'develop' into report_weight_in_yen

77a304f

Merge branch 'report_weight_in_yen' into enum_cycle_undirected

75f3cf1

fix styles

0418243

dcoudert reviewed Jun 15, 2025

View reviewed changes

dcoudert approved these changes Jun 15, 2025

View reviewed changes

dcoudert added s: positive review and removed s: needs review labels Jun 15, 2025

kappybar mentioned this pull request Jun 22, 2025

PNC k shortest simple path (for directed graphs) #40284

Merged

5 tasks

vbraun merged commit 720a7f6 into sagemath:develop Jun 25, 2025
24 of 28 checks passed

github-actions Bot removed the s: positive review label Jun 25, 2025

kappybar deleted the enum_cycle_undirected branch September 10, 2025 19:27

Uh oh!

Conversation

kappybar commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Checklist

⌛ Dependencies

Uh oh!

github-actions Bot commented Jun 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dcoudert Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

dcoudert Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

dcoudert Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

dcoudert Jun 9, 2025

Choose a reason for hiding this comment

Uh oh!

dcoudert left a comment

Choose a reason for hiding this comment

Uh oh!

dcoudert Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

dcoudert Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

dcoudert left a comment

Choose a reason for hiding this comment

Uh oh!

dcoudert commented Jun 15, 2025

Uh oh!

kappybar commented Jun 15, 2025

Uh oh!

dcoudert commented Jun 15, 2025

Uh oh!

kappybar commented Jun 15, 2025

Uh oh!

dcoudert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kappybar commented Jun 6, 2025 •

edited

Loading

github-actions Bot commented Jun 6, 2025 •

edited

Loading